feat: made a new password input component #108
Conversation
|
Preview is ready. |
src/components/Password/Password.tsx
Outdated
| TextInput, | ||
| TextInputProps, | ||
| } from '@gravity-ui/uikit'; | ||
| import _ from 'lodash'; |
| } | ||
|
|
||
| &__button { | ||
| --yc-button-background-color-hover: transparent; |
There was a problem hiding this comment.
Technically this is not part of public API. Moreover this var will be removed in the next major
| /** Show copy button */ | ||
| showCopyButton?: boolean; | ||
| /** Show visibility button */ | ||
| showVisibilityButton?: boolean; |
There was a problem hiding this comment.
Lets call it showRevealButton
| <div className={b('additional-right-content')}> | ||
| {rightContent} | ||
| {value && showCopyButton ? ( | ||
| <CopyToClipboard text={String(value)} timeout={500}> |
There was a problem hiding this comment.
Can be replaced with ClipboardButton component, which have tooltip functionality
| view="flat-secondary" | ||
| onClick={onClick} | ||
| className={b('button')} | ||
| size="s" |
There was a problem hiding this comment.
We should use different sizes for each TextInput size
| ) : null} | ||
| {showVisibilityButton ? ( | ||
| <Button | ||
| view="flat-secondary" |
There was a problem hiding this comment.
aria-label prop would be nice here with the texts: Show password/Hide password
| type={hideValue ? 'password' : 'text'} | ||
| rightContent={additionalRightContent} | ||
| autoComplete={autoComplete ? autoComplete : 'new-password'} | ||
| className={b('input', className)} |
There was a problem hiding this comment.
That's not the input's className, you should pass it to the controlProps prop
| /** Show reveal button */ | ||
| showRevealButton?: boolean; | ||
| /** Disable tooltip. Tooltip won't be shown */ | ||
| hasTooltip?: boolean; |
There was a problem hiding this comment.
I would pick separate names here, hasCopyTooltip and hasRevealTooltip
| <ClipboardButton | ||
| text={value} | ||
| hasTooltip={hasTooltip} | ||
| size={16} |
There was a problem hiding this comment.
We should map input size to button size
| <Button | ||
| view="flat-secondary" | ||
| onClick={onClick} | ||
| size={size} |
There was a problem hiding this comment.
TextInput size !== Button size, take a look at clear button, maybe reuse this logic here as well
|
|
||
| return ( | ||
| <TextInput | ||
| {...props} | ||
| type={hideValue ? 'password' : 'text'} | ||
| rightContent={additionalRightContent} | ||
| autoComplete={autoComplete ? autoComplete : 'new-password'} | ||
| className={b('input', className)} | ||
| controlProps={{ | ||
| className: b(), |
There was a problem hiding this comment.
Now it's an opposite, you're passing root className (b()) to the control node
| return ( | ||
| <div className={b()}> | ||
| <PasswordInput {...props} onUpdate={setValue} value={value} /> | ||
| <Button onClick={generateRandomValue} className={b('button-generate-random-value')}> |
There was a problem hiding this comment.
Can you add prop showCopyButton and showRevealButton?
| @@ -86,7 +102,8 @@ export const PasswordInput: React.FC<PasswordInputProps> = (props) => { | |||
| rightContent={additionalRightContent} | |||
| autoComplete={autoComplete ? autoComplete : 'new-password'} | |||
| controlProps={{ | |||
| className: b(), | |||
| className: b('input-control', controlProps?.className), | |||
| ...controlProps, | |||
There was a problem hiding this comment.
Spread should be before the className, otherwise it would be overwritten by the controlProps
amje
changed the title
No description provided.